ErrorInstance: keep the captured stack frames alive until the error info is materialized - #511

JSC held the frames captured by an ErrorInstance weakly. When a callee or code block in the trace died before the first .stack read, the GC end phase pre-rendered the stack string through a callback that gets no error instance and cannot run JS. The cached string had a bare "Error" header with no message, Error.prepareStackTrace never ran, and no call sites existed. Any error created inside a function object that is collected before the read hit this: an IIFE, a .then callback, the prologue of an async function, or a module's top-level code before its first await.

oven-sh/WebKit#511 makes ErrorInstance::visitChildren visit the frames under the cell lock, so the first .stack read always sees live frames. This takes the cell lock at the two places where Bun mutates the frame vector of an ErrorInstance, adds the write barrier that Error.appendStackTrace needs now that the destination keeps the appended frames alive, and points WEBKIT_VERSION at the preview build of that change.

JSC held the frames captured by an ErrorInstance weakly. When a callee or code block in the trace died before the first .stack read, the GC end phase pre-rendered the stack string through a callback that gets no error instance and cannot run JS. The cached string had a bare "Error" header with no message, Error.prepareStackTrace never ran, and no call sites existed. Any error created inside a function object that is collected before the read hit this: an IIFE, a .then callback, the prologue of an async function, or a module's top-level code before its first await.

oven-sh/WebKit#511 makes ErrorInstance::visitChildren visit the frames under the cell lock, so the first .stack read always sees live frames. This takes the cell lock at the two places where Bun mutates the frame vector of an ErrorInstance, adds the write barrier that Error.appendStackTrace needs now that the destination keeps the appended frames alive, and points WEBKIT_VERSION at the preview build of that change.

…#42548)

### Problem
- A synchronous GC (`Bun.gc(true)`, a heap snapshot) inside
`Error.prepareStackTrace` crashes the first `.stack` read of an error
whose creating function is already garbage. Release: `panic(main
thread): Segmentation fault at address 0x8`. Debug: `ASSERTION FAILED:
!m_errorInfoMaterialized` in `ErrorInstance::computeErrorInfo`
(`ErrorInstance.cpp:398`).
- `materializeErrorInfoIfNeeded` (WebKit fork, `ErrorInstance.cpp:441`)
sets `m_errorInfoMaterialized`, then calls Bun's formatter with
`*m_stackTrace` still installed. A GC in there finds a dead frame, so
`reconcileWeakReferencesAtGCEnd` renders the trace and frees the vector.
The `m_stackTrace->clear()` that follows dereferences null.
- A GC-stress fuzz run found this. There is no user report.

### Fix
- `computeErrorInfoWrapperToJSValue`
(`src/jsc/bindings/FormatStackTraceForJS.cpp`) roots every frame's
callee and code block in a `MarkedArgumentBuffer` while it formats, as
the lazy `Error.captureStackTrace` getter already does (#31495). This
covers the callback, a `message` getter and a `node:vm`
`Error.prepareStackTrace` getter.
- `Error.appendStackTrace` returns early for a destination flagged as
materialized (the guard from #37370). Inside the callback it appended
unrooted frames to the vector being formatted.
- The lazy getter can run re-entrantly for the error being formatted. It
now copies the frames and leaves the vector installed. It freed the
vector that the `node:vm` path still reads.
- Verified: `test/js/node/v8/capture-stack-trace.test.js`, 6 new cases,
all fail on 1.4.3 canary and on a debug build of main. Self-reviewed: 3
concerns raised, 3 addressed.

### Background
- `error.stack` is lazy. `new Error()` stores a `Vector<StackFrame>`.
The first read calls Bun's formatter, which builds `CallSite` objects
and calls `Error.prepareStackTrace`.
- The error holds each frame's callee and code block weakly.
`reconcileWeakReferencesAtGCEnd` runs on every live error at the end of
each GC.
- A `MarkedArgumentBuffer` is a stack-allocated list of GC roots.

<details><summary>Notes</summary>

Minimal script (prints `formatted` on Node and with this change):

```js
const e = new Function('"use strict"; function inner() { try { return new Error("x"); } finally {} } try { return inner(); } finally {}')();
Error.prepareStackTrace = (err, callSites) => { Bun.gc(true); return "formatted"; };
console.log(e.stack);
```

A more ordinary shape that crashes the same way (ESM):

```js
import v8 from "node:v8";
Error.prepareStackTrace = (err, callSites) => { v8.getHeapStatistics(); return "formatted " + callSites.length; };
async function handler() {
  const e = (() => { try { return new Error("request failed"); } finally {} })();
  await 1;
  return e.stack;
}
console.log(await handler());
```

The `finally` blocks keep each `return` out of tail position. A
`CallSite` for a strict-mode frame does not keep the function (the V8
rule for `getFunction()`), so nothing else keeps such a frame alive
during the callback. A sloppy-mode trace does not crash, because its
call sites retain the functions.

`materializeErrorInfoIfNeeded` holds a `DeferGCForAWhile`, so only an
explicit synchronous collection runs in the formatter. Calls that
collect synchronously and reach this: `Bun.gc(true)`, `bun:jsc`
`fullGC()` and `edenGC()`, `Bun.generateHeapSnapshot()`,
`v8.writeHeapSnapshot()`. `v8.getHeapStatistics()` and `bun:jsc`
`heapStats()` collect only while `vm.heap.size() == 0`
(`src/jsc/modules/BunJSCModule.h:233`), so they reach this only before
the first collection of the process. `Bun.gc(false)` and allocation
pressure do not reach it.

The lazy getter case needs no GC. `Error.captureStackTrace(e)` on an
error from a `node:vm` context installs the lazy `stack` accessor. The
first `e.stack` read runs `materializeErrorInfoIfNeeded`, whose
formatter reads the context's `Error.prepareStackTrace`. If that is a
getter that reads `e.stack` again, `materializeErrorInfoIfNeeded`
returns at once (the flag is set) and the accessor runs
`errorInstanceLazyStackCustomGetter` for the same error. It moved the
frames out and called `setStackFrames(vm, {})`, which deleted the vector
the outer formatter holds by reference. ASAN with `Malloc=1` reports
`heap-use-after-free` in `Bun::formatStackTrace`
(`FormatStackTraceForJS.cpp:168`). A release build reads a size of zero
and prints a trace with no frames. `Reflect.get(otherError, "stack", e)`
reaches the same getter with `e` as the receiver.

This PR does not close every way to reach
`ASSERT(!m_errorInfoMaterialized)`. #37370 covers
`Error.appendStackTrace` onto a destination whose `.stack` was read
earlier, outside the callback, with its own tests, plus two other
`appendStackTrace` crashes. The early return here is the same hunk.
Whichever PR lands second needs a trivial rebase.

Relation to #40354 with oven-sh/WebKit#511: that change makes
`ErrorInstance::visitChildren` mark the frames while `m_stackTrace` is
installed. The vector is still installed during the callback, so it
would also keep the frames marked there and make the buffer here
redundant but harmless. It is a retention change with a WebKit pin bump,
and it has no test for this crash. The cases here stay valid as its
regression coverage.

A change on the WebKit side alone is not enough. If
`materializeErrorInfoIfNeeded` moves the vector out of the instance
before the call, the GC end phase skips the error, but the frames are
then unrooted and the `node:vm` path reads them after user JS ran. If
`reconcileWeakReferencesAtGCEnd` skips materialized instances,
`Bun.inspect(err)` inside the callback reads dead frames through
`stackTrace()`. Both shapes still need the frames rooted for the
duration of the call.

Cost: one `MarkedArgumentBuffer` per first `.stack` read, two slots per
frame. It spills to the heap above four frames.

Suites run on the debug build: `capture-stack-trace.test.js` (52 pass,
also with `BUN_JSC_validateExceptionChecks=1`), `stack.test.ts`,
`prepare-stack-trace-crash.test.ts`, `fix-bindings-stack-trace.test.ts`,
`inspect-error.test.js`, `node/vm/vm.test.ts`,
`external-sourcemap-stack-leak.test.ts`, and the Node tests
`test-error-prepare-stack-trace.js`,
`test-util-getcallsites-preparestacktrace.js`,
`test-shadow-realm-prepare-stack-trace.js`.
</details>

…oven-sh#42548)

### Problem
- A synchronous GC (`Bun.gc(true)`, a heap snapshot) inside
`Error.prepareStackTrace` crashes the first `.stack` read of an error
whose creating function is already garbage. Release: `panic(main
thread): Segmentation fault at address 0x8`. Debug: `ASSERTION FAILED:
!m_errorInfoMaterialized` in `ErrorInstance::computeErrorInfo`
(`ErrorInstance.cpp:398`).
- `materializeErrorInfoIfNeeded` (WebKit fork, `ErrorInstance.cpp:441`)
sets `m_errorInfoMaterialized`, then calls Bun's formatter with
`*m_stackTrace` still installed. A GC in there finds a dead frame, so
`reconcileWeakReferencesAtGCEnd` renders the trace and frees the vector.
The `m_stackTrace->clear()` that follows dereferences null.
- A GC-stress fuzz run found this. There is no user report.

### Fix
- `computeErrorInfoWrapperToJSValue`
(`src/jsc/bindings/FormatStackTraceForJS.cpp`) roots every frame's
callee and code block in a `MarkedArgumentBuffer` while it formats, as
the lazy `Error.captureStackTrace` getter already does (oven-sh#31495). This
covers the callback, a `message` getter and a `node:vm`
`Error.prepareStackTrace` getter.
- `Error.appendStackTrace` returns early for a destination flagged as
materialized (the guard from oven-sh#37370). Inside the callback it appended
unrooted frames to the vector being formatted.
- The lazy getter can run re-entrantly for the error being formatted. It
now copies the frames and leaves the vector installed. It freed the
vector that the `node:vm` path still reads.
- Verified: `test/js/node/v8/capture-stack-trace.test.js`, 6 new cases,
all fail on 1.4.3 canary and on a debug build of main. Self-reviewed: 3
concerns raised, 3 addressed.

### Background
- `error.stack` is lazy. `new Error()` stores a `Vector<StackFrame>`.
The first read calls Bun's formatter, which builds `CallSite` objects
and calls `Error.prepareStackTrace`.
- The error holds each frame's callee and code block weakly.
`reconcileWeakReferencesAtGCEnd` runs on every live error at the end of
each GC.
- A `MarkedArgumentBuffer` is a stack-allocated list of GC roots.

<details><summary>Notes</summary>

Minimal script (prints `formatted` on Node and with this change):

```js
const e = new Function('"use strict"; function inner() { try { return new Error("x"); } finally {} } try { return inner(); } finally {}')();
Error.prepareStackTrace = (err, callSites) => { Bun.gc(true); return "formatted"; };
console.log(e.stack);
```

A more ordinary shape that crashes the same way (ESM):

```js
import v8 from "node:v8";
Error.prepareStackTrace = (err, callSites) => { v8.getHeapStatistics(); return "formatted " + callSites.length; };
async function handler() {
  const e = (() => { try { return new Error("request failed"); } finally {} })();
  await 1;
  return e.stack;
}
console.log(await handler());
```

The `finally` blocks keep each `return` out of tail position. A
`CallSite` for a strict-mode frame does not keep the function (the V8
rule for `getFunction()`), so nothing else keeps such a frame alive
during the callback. A sloppy-mode trace does not crash, because its
call sites retain the functions.

`materializeErrorInfoIfNeeded` holds a `DeferGCForAWhile`, so only an
explicit synchronous collection runs in the formatter. Calls that
collect synchronously and reach this: `Bun.gc(true)`, `bun:jsc`
`fullGC()` and `edenGC()`, `Bun.generateHeapSnapshot()`,
`v8.writeHeapSnapshot()`. `v8.getHeapStatistics()` and `bun:jsc`
`heapStats()` collect only while `vm.heap.size() == 0`
(`src/jsc/modules/BunJSCModule.h:233`), so they reach this only before
the first collection of the process. `Bun.gc(false)` and allocation
pressure do not reach it.

The lazy getter case needs no GC. `Error.captureStackTrace(e)` on an
error from a `node:vm` context installs the lazy `stack` accessor. The
first `e.stack` read runs `materializeErrorInfoIfNeeded`, whose
formatter reads the context's `Error.prepareStackTrace`. If that is a
getter that reads `e.stack` again, `materializeErrorInfoIfNeeded`
returns at once (the flag is set) and the accessor runs
`errorInstanceLazyStackCustomGetter` for the same error. It moved the
frames out and called `setStackFrames(vm, {})`, which deleted the vector
the outer formatter holds by reference. ASAN with `Malloc=1` reports
`heap-use-after-free` in `Bun::formatStackTrace`
(`FormatStackTraceForJS.cpp:168`). A release build reads a size of zero
and prints a trace with no frames. `Reflect.get(otherError, "stack", e)`
reaches the same getter with `e` as the receiver.

This PR does not close every way to reach
`ASSERT(!m_errorInfoMaterialized)`. oven-sh#37370 covers
`Error.appendStackTrace` onto a destination whose `.stack` was read
earlier, outside the callback, with its own tests, plus two other
`appendStackTrace` crashes. The early return here is the same hunk.
Whichever PR lands second needs a trivial rebase.

Relation to oven-sh#40354 with oven-sh/WebKit#511: that change makes
`ErrorInstance::visitChildren` mark the frames while `m_stackTrace` is
installed. The vector is still installed during the callback, so it
would also keep the frames marked there and make the buffer here
redundant but harmless. It is a retention change with a WebKit pin bump,
and it has no test for this crash. The cases here stay valid as its
regression coverage.

A change on the WebKit side alone is not enough. If
`materializeErrorInfoIfNeeded` moves the vector out of the instance
before the call, the GC end phase skips the error, but the frames are
then unrooted and the `node:vm` path reads them after user JS ran. If
`reconcileWeakReferencesAtGCEnd` skips materialized instances,
`Bun.inspect(err)` inside the callback reads dead frames through
`stackTrace()`. Both shapes still need the frames rooted for the
duration of the call.

Cost: one `MarkedArgumentBuffer` per first `.stack` read, two slots per
frame. It spills to the heap above four frames.

Suites run on the debug build: `capture-stack-trace.test.js` (52 pass,
also with `BUN_JSC_validateExceptionChecks=1`), `stack.test.ts`,
`prepare-stack-trace-crash.test.ts`, `fix-bindings-stack-trace.test.ts`,
`inspect-error.test.js`, `node/vm/vm.test.ts`,
`external-sourcemap-stack-leak.test.ts`, and the Node tests
`test-error-prepare-stack-trace.js`,
`test-util-getcallsites-preparestacktrace.js`,
`test-shadow-realm-prepare-stack-trace.js`.
</details>